Refactor Lily58 to use split_common #6260

yyc · 2019-07-05T13:17:06Z

Now that split_common is pretty mature, it might be a good idea to get rid of all the duplicated functionality and just use the common libraries so we can piggyback on those updates.

Description

This removes the duplicated code (matrix.c, split_util.c, etc.), and updates the build files to automatically include the split_common files.

Then, the OLED code is refactored to use the core OLED library, so we can remove ssd1306.c as well. All the widgets are also updated for compatibility with the new core OLED library. The default keymap has also been updated to work with the core and OLED libraries. For a guide on how to update your own keymap for compatibility, take a look at the default keymap refactor.

This is tested to work with two halves communicating over serial, with i2c communication on both OLEDs. (sidenote: does i2c over the two halves even work? I couldn't get it to work even with the original repo)

My build doesn't have ws2812 underlighting, so I'm unable to test whether that's working. Help on that would be appreciated.

Types of Changes

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

…e default to match

yyc · 2019-07-05T13:22:19Z

@kata0510 would appreciate you taking a look at this :)

@drashna addresses some of your concerns with the Helix code that was originally used

ghost · 2019-07-06T05:30:54Z

I'll test encoder functionality, backlighting, and anything else I can think of, and report back with any issues I find.

drashna

To be blunt, it may be much better to move the OLED code all into the keymap.c file, so that it's accessible and editable there, rather than relying on the "lib" stuff.

This is especially true of the layer code.

Also, why not 270 degree rotation? :)

keyboards/lily58/config.h

drashna · 2019-07-06T06:07:03Z

This is tested to work with two halves communicating over serial, with i2c communication on both OLEDs. (sidenote: does i2c over the two halves even work? I couldn't get it to work even with the original repo)

If this is like the helix, I think you have to cut a trace (between two pads), and add 2x resistors on one of the sides for it to work properly.

Additionally, you can't have OLED's on both sides in this case, since they use the same i2c address, which is a big no-no.

ghost · 2019-07-06T06:13:21Z

OLEDs appear to be completely non-functional
Encoder doesn't appear to work on either side of the keyboard using the same changes as work on the main branch, which include enabling encoders, defining the pads, defining the resolution, and adding in the function to monitor the encoder.
RGB doesn't appear to work - triggering RGB_MOD doesn't do anything, and adding custom rgb code in the keymap.c file as I do on the main branch results in implicit declaration errors of rgblight_enable and rgblight_sethsv

yyc · 2019-07-06T08:04:24Z

@MysticMixles

Do the OLEDs not work even with the default keymap? Or if you're using your own keymap, make sure to add OLED_DRIVER_ENABLE= yes to your rules.mk
can I see your code for enabling the encoder? I have an encoder lying around somewhere and I can test with that. (sidenote: I'm interested to see how your encoder is integrated into your keyboard, if you won't mind sharing 😃 )
For RGB, can you try directly calling rgblight_enable_noeeprom() in your keymap's matrix_init_user function? I don't think RGB_MOD will turn on the LEDs if they're off.

ghost · 2019-07-06T08:56:29Z

@ycc

Do the OLEDs not work even with the default keymap? Or if you're using your own keymap, make sure to add OLED_DRIVER_ENABLE= yes to your rules.mk

I just added OLED_DRIVER_ENABLE= yes to my rules.mk, but both OLEDs are still turned off.

can I see your code for enabling the encoder? I have an encoder lying around somewhere and I can test with that. (sidenote: I'm interested to see how your encoder is integrated into your keyboard, if you won't mind sharing 😃 )

void encoder_update_user(uint8_t index, bool clockwise) { if (index == 0) { if (clockwise) { tap_code(KC_VOLU); } else { tap_code(KC_VOLD); } } }

This is the code I'm using and it works, assuming the encoder is on the master side - the main branch's custom split code doesn't let an encoder work on the slave side at all, as far as I can tell.

For RGB, can you try directly calling rgblight_enable_noeeprom() in your keymap's matrix_init_user function? I don't think RGB_MOD will turn on the LEDs if they're off.

I didn't realize that rgblight wasn't enabled by default, I just turned it on and it works. However, if I call rgblight_sethsv, I still get errors that it hasn't been defined.

ghost · 2019-07-06T08:58:38Z

Also, the RGB only works on the master side of the keyboard - whatever side I plug the usb cable into lights up, and the other side is dark.

ghost · 2019-07-06T09:20:23Z

Looking at the code, I'm not super familiar with QMK, but I actually don't see where the OLED gets turned on - in the main branch there is a #define SSD1306OLED, and in the keymap code there is some OLED code, and in this branch there isn't any - I don't know if this is causing the issue, or just because of the split common changes, but it is the first thing I jumped to.

ghost · 2019-07-06T22:55:37Z

Oh, and for installing the encoder, I used these, and bent all of the pins to be flat with the pcb, other than the mounting legs, which I put into the holes that a PCB mount switch would use. This holds pretty tight, and probably isn't great for the keyboard if you reinstall several times, but works fine for me.

drashna · 2019-07-08T19:16:05Z

@MysticMixles I believe that for the SSD1306 type code, it gets turned on by keypress, and statup.

ghost · 2019-07-08T21:04:24Z

@MysticMixles I believe that for the SSD1306 type code, it gets turned on by keypress, and statup.

Then it is broken - when I flashed the main branch the screens worked as expected

drashna · 2019-07-08T21:06:47Z

Well, for the OLED Driver feature, it runs it from keyboard_task, and is triggered by a matrix change. That's why it doesn't work right on the non-split common stuff.

yyc · 2019-07-10T10:24:05Z

Looking at the code, I'm not super familiar with QMK, but I actually don't see where the OLED gets turned on - in the main branch there is a #define SSD1306OLED, and in the keymap code there is some OLED code, and in this branch there isn't any - I don't know if this is causing the issue, or just because of the split common changes, but it is the first thing I jumped to.

As long as OLED_DRIVER_ENABLE is set and oled_task_user(void) is defined, the OLEDs should be set up automatically. It's in the oled driver documentation. Can you try using the default keymap in this branch and see if that works for you?

Also, the RGB only works on the master side of the keyboard - whatever side I plug the usb cable into lights up, and the other side is dark.

Typically you would wire the RGBs the other way round, and have the LED data pin be wired to the unused TRRS connector, so that the master would do all the work of controlling the RGBs. In this case, since you've wired the data pin on both sides to the pro micro, you would probably need to enable RGB_SPLIT documented here to get it working.

Disclaimer: my lily58 isn't wired with RGBs so I have no way of testing this, but I have gotten the same setup working with an iris split keyboard.

re: the encoders, still trying to get encoders working with my keyboard. I'll keep you updated.

yyc · 2019-07-10T10:42:37Z

void encoder_update_user(uint8_t index, bool clockwise) { if (index == 0) { if (clockwise) { tap_code(KC_VOLU); } else { tap_code(KC_VOLD); } } }

Actually, according to this thread, if your encoder is on the slave side you should change the index to 1 (or if you have just one encoder, get rid of the index-checking altogether and just have:

void encoder_update_user(uint8_t index, bool clockwise) {
  if (clockwise) { tap_code(KC_VOLU); }
  else { tap_code(KC_VOLD); }
}

ghost · 2019-07-10T10:48:33Z

@yyc The encoder doesn't work on either side of the keyboard, using the default keymap with that bit of code added and encoders enabled. I haven't tried the no index method, but it doesn't work on the master side with an index of either 1 or 0.

I'll play with RGB_SPLIT in a few hours to see if that works.

Regarding the OLEDs, using the default keymap they don't appear to work.

ghost · 2019-07-10T20:42:32Z

I still can't call rgblight_sethsv - it is undefined, even after adding RGB_SPLIT.

Also, using the index-less encoder code, it doesn't work either.

And I'm unclear as to what's going on with the OLEDs - are they supposed to work, or do I need to change things? They don't turn on using the default keymap.

yyc · 2019-07-14T02:54:58Z

@MysticMixles

I still can't call rgblight_sethsv - it is undefined, even after adding RGB_SPLIT.

Can you upload a copy of your repo?

Also, using the index-less encoder code, it doesn't work either.

I've tested the split encoder code individually and it seems to work but not when I plug everything in so I'm not sure whether there's something wrong with my testing setup or whether there's some low-level issue that I'm not thinking of :/

And I'm unclear as to what's going on with the OLEDs - are they supposed to work, or do I need to change things? They don't turn on using the default keymap.

OLEDs should work immediately when you flash with the default keymap.
You said your OLEDs work on the master branch default keymap without any changes?

yanfali · 2019-07-31T15:34:41Z

If I have some time later today, I'll give this a try, I have the lily58 pro with oleds and there's an rgb strip but it's not wired up at the moment.

noroadsleft · 2020-05-03T07:55:36Z

keyboards/lily58/lib/glcdfont_lily.c

@@ -1,6 +1,10 @@
 // This is the 'classic' fixed-space bitmap font for Adafruit_GFX since 1.0.
 // See gfxfont.h for newer custom bitmap font info.

+// Modified to show the Lily58 logo instead of the qmk logo
+
+#pragma once


Have never known #pragma once to be used in a .c file 😕

¯\(ツ)/¯
was requested by drashna in #6260 (comment)

well this is used in an #include by the OLED driver, should not be a problem to rename it to .h

@drashna 😕

noroadsleft

Flashed the default keymap from this branch to some Pro Micros. Don't have the actual PCB or the hardware to emulate this board, so __attribute__ ((weak)) ✔️ because the only thing I can test is that it works as a keyboard.

keyboards/lily58/rules.mk

keyboards/lily58/lib/keylogger.c

keyboards/lily58/lib/host_led_state_reader.c

keyboards/lily58/lib/glcdfont_lily.c

keyboards/lily58/keymaps/chuan/keymap.c

keyboards/lily58/lib/timelogger.c

keyboards/lily58/keymaps/chuan/rules.mk

Co-authored-by: Ryan <fauxpark@gmail.com>

noroadsleft · 2020-05-04T00:05:45Z

Looks good to me.

@yyc Fantastic work on the changelog entry. ❤️

noroadsleft · 2020-05-04T00:06:36Z

Thanks!

@ninjonas

* Refactor core keyboard files to use split_common * Refactor OLED libs for compatibility with split_common * Override default glcd font to get lily58 logo * Switch to OLED library, clean up references to SSD1306OLED, and update default to match * Remove duplicated #define tapping_term * Add in logo_reader that was accidentally deleted * fix yuchi keymap as well to get CI passing * remove serial_config * keyboards/lily58/lib/layer_state_reader.c * incorporate layer changes * add my own lily58 keymap * incorporate suggestions and fix CI warnings * Remove reference to SSD1306OLED in @ninjonas' build * Update ninjonas.c * fix build/import issues * update km * update debounce * update DIODE_DIRECTION * address comments * address comments * add Changelog * Apply suggestions

@ninjonas

* Refactor core keyboard files to use split_common * Refactor OLED libs for compatibility with split_common * Override default glcd font to get lily58 logo * Switch to OLED library, clean up references to SSD1306OLED, and update default to match * Remove duplicated #define tapping_term * Add in logo_reader that was accidentally deleted * fix yuchi keymap as well to get CI passing * remove serial_config * keyboards/lily58/lib/layer_state_reader.c * incorporate layer changes * add my own lily58 keymap * incorporate suggestions and fix CI warnings * Remove reference to SSD1306OLED in @ninjonas' build * Update ninjonas.c * fix build/import issues * update km * update debounce * update DIODE_DIRECTION * address comments * address comments * add Changelog * Apply suggestions

Pursuant qmk/qmk_firmware#6260 Update to take advantage of refactor (and OLED driver). Next up: - Figure out custom logo - Work on OLED configuration

@ninjonas

* Refactor core keyboard files to use split_common * Refactor OLED libs for compatibility with split_common * Override default glcd font to get lily58 logo * Switch to OLED library, clean up references to SSD1306OLED, and update default to match * Remove duplicated #define tapping_term * Add in logo_reader that was accidentally deleted * fix yuchi keymap as well to get CI passing * remove serial_config * keyboards/lily58/lib/layer_state_reader.c * incorporate layer changes * add my own lily58 keymap * incorporate suggestions and fix CI warnings * Remove reference to SSD1306OLED in @ninjonas' build * Update ninjonas.c * fix build/import issues * update km * update debounce * update DIODE_DIRECTION * address comments * address comments * add Changelog * Apply suggestions

* Branch point for 2020 May 30 Breaking Change * Migrate `ACTION_LAYER_TOGGLE` to `TG()` (#8954) * Migrate `ACTION_MODS_ONESHOT` to `OSM()` (#8957) * Migrate `ACTION_DEFAULT_LAYER_SET` to `DF()` (#8958) * Migrate `ACTION_LAYER_MODS` to `LM()` (#8959) * Migrate `ACTION_MODS_TAP_KEY` to `MT()` (#8968) * Convert V-USB usbdrv to a submodule (#8321) * Unify Tap Hold functions and documentation (#8348) * Changing board names to prevent confusion (#8412) * Move the Keyboardio Model01 to a keyboardio/ subdir (#8499) * Move spaceman keyboards (#8830) * Migrate miscellaneous `fn_actions` entries (#8977) * Migrate `ACTION_MODS_KEY` to chained mod keycodes (#8979) * Organizing my keyboards (plaid, tartan, ergoinu) (#8537) * Refactor Lily58 to use split_common (#6260) * Refactor zinc to use split_common (#7114) * Add a message if bin/qmk doesn't work (#9000) * Fix conflicting types for 'tfp_printf' (#8269) * Fixed RGB_DISABLE_AFTER_TIMEOUT to be seconds based & small internals cleanup (#6480) * Refactor and updates to TKC1800 code (#8472) * Switch to qmk forks for everything (#9019) * audio refactor: replace deprecated PLAY_NOTE_ARRAY (#8484) * Audio enable corrections (2/3) (#8903) * Split HHKB to ANSI and JP layouts and Add VIA support for each (#8582) * Audio enable corrections (Part 4) (#8974) * Fix typo from PR7114 (#9171) * Augment future branch Changelogs (#8978) * Revert "Branch point for 2020 May 30 Breaking Change"

* Branch point for 2020 May 30 Breaking Change * Migrate `ACTION_LAYER_TOGGLE` to `TG()` (qmk#8954) * Migrate `ACTION_MODS_ONESHOT` to `OSM()` (qmk#8957) * Migrate `ACTION_DEFAULT_LAYER_SET` to `DF()` (qmk#8958) * Migrate `ACTION_LAYER_MODS` to `LM()` (qmk#8959) * Migrate `ACTION_MODS_TAP_KEY` to `MT()` (qmk#8968) * Convert V-USB usbdrv to a submodule (qmk#8321) * Unify Tap Hold functions and documentation (qmk#8348) * Changing board names to prevent confusion (qmk#8412) * Move the Keyboardio Model01 to a keyboardio/ subdir (qmk#8499) * Move spaceman keyboards (qmk#8830) * Migrate miscellaneous `fn_actions` entries (qmk#8977) * Migrate `ACTION_MODS_KEY` to chained mod keycodes (qmk#8979) * Organizing my keyboards (plaid, tartan, ergoinu) (qmk#8537) * Refactor Lily58 to use split_common (qmk#6260) * Refactor zinc to use split_common (qmk#7114) * Add a message if bin/qmk doesn't work (qmk#9000) * Fix conflicting types for 'tfp_printf' (qmk#8269) * Fixed RGB_DISABLE_AFTER_TIMEOUT to be seconds based & small internals cleanup (qmk#6480) * Refactor and updates to TKC1800 code (qmk#8472) * Switch to qmk forks for everything (qmk#9019) * audio refactor: replace deprecated PLAY_NOTE_ARRAY (qmk#8484) * Audio enable corrections (2/3) (qmk#8903) * Split HHKB to ANSI and JP layouts and Add VIA support for each (qmk#8582) * Audio enable corrections (Part 4) (qmk#8974) * Fix typo from PR7114 (qmk#9171) * Augment future branch Changelogs (qmk#8978) * Revert "Branch point for 2020 May 30 Breaking Change"

yyc added 6 commits July 5, 2019 18:20

Refactor core keyboard files to use split_common

073b9ef

Refactor OLED libs for compatibility with split_common

0808bfa

Override default glcd font to get lily58 logo

43207e9

Switch to OLED library, clean up references to SSD1306OLED, and updat…

3d6ae58

…e default to match

Remove duplicated #define tapping_term

651463b

Add in logo_reader that was accidentally deleted

279b482

fix yuchi keymap as well to get CI passing

4c154e2

yyc mentioned this pull request Jul 6, 2019

Lily58 can't have an encoder on slave side #6257

Closed

drashna requested changes Jul 6, 2019

View reviewed changes

keyboards/lily58/config.h Outdated Show resolved Hide resolved

remove serial_config

ee19c87

drashna added the keyboard label Jul 8, 2019

drashna requested a review from a team July 30, 2019 08:13

drashna approved these changes May 3, 2020

View reviewed changes

noroadsleft reviewed May 3, 2020

View reviewed changes

noroadsleft approved these changes May 3, 2020

View reviewed changes

fauxpark requested changes May 3, 2020

View reviewed changes

Apply suggestions

45a8c23

Co-authored-by: Ryan <fauxpark@gmail.com>

noroadsleft merged commit f655d09 into qmk:future May 4, 2020

berfarah added a commit to berfarah/keyboard-layouts that referenced this pull request May 26, 2020

lily58: Update to use OLED driver

8c030db

Pursuant qmk/qmk_firmware#6260 Update to take advantage of refactor (and OLED driver). Next up: - Figure out custom logo - Work on OLED configuration

berfarah mentioned this pull request May 26, 2020

lily58: Update to use OLED driver berfarah/keyboard-layouts#1

Merged

dancarroll mentioned this pull request Mar 2, 2021

Remove more cruft from Lily58 default keymap #12078

Merged

14 tasks

tzarc mentioned this pull request Mar 31, 2021

Remove leftover old implementations of the lily58 split code. #12442

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Lily58 to use split_common #6260

Refactor Lily58 to use split_common #6260

yyc commented Jul 5, 2019 •

edited by noroadsleft

Loading

yyc commented Jul 5, 2019

ghost commented Jul 6, 2019

drashna left a comment

drashna commented Jul 6, 2019

ghost commented Jul 6, 2019

yyc commented Jul 6, 2019

ghost commented Jul 6, 2019

ghost commented Jul 6, 2019

ghost commented Jul 6, 2019

ghost commented Jul 6, 2019 •

edited by ghost

Loading

drashna commented Jul 8, 2019

ghost commented Jul 8, 2019

drashna commented Jul 8, 2019

yyc commented Jul 10, 2019

yyc commented Jul 10, 2019

ghost commented Jul 10, 2019

ghost commented Jul 10, 2019

yyc commented Jul 14, 2019

yanfali commented Jul 31, 2019

noroadsleft May 3, 2020

yyc May 3, 2020 •

edited

Loading

rhruiz May 3, 2020

noroadsleft May 4, 2020

noroadsleft left a comment

noroadsleft commented May 4, 2020

noroadsleft commented May 4, 2020

Refactor Lily58 to use split_common #6260

Refactor Lily58 to use split_common #6260

Conversation

yyc commented Jul 5, 2019 • edited by noroadsleft Loading

Description

Types of Changes

Checklist

yyc commented Jul 5, 2019

ghost commented Jul 6, 2019

drashna left a comment

Choose a reason for hiding this comment

drashna commented Jul 6, 2019

ghost commented Jul 6, 2019

yyc commented Jul 6, 2019

ghost commented Jul 6, 2019

ghost commented Jul 6, 2019

ghost commented Jul 6, 2019

ghost commented Jul 6, 2019 • edited by ghost Loading

drashna commented Jul 8, 2019

ghost commented Jul 8, 2019

drashna commented Jul 8, 2019

yyc commented Jul 10, 2019

yyc commented Jul 10, 2019

ghost commented Jul 10, 2019

ghost commented Jul 10, 2019

yyc commented Jul 14, 2019

yanfali commented Jul 31, 2019

noroadsleft May 3, 2020

Choose a reason for hiding this comment

yyc May 3, 2020 • edited Loading

Choose a reason for hiding this comment

rhruiz May 3, 2020

Choose a reason for hiding this comment

noroadsleft May 4, 2020

Choose a reason for hiding this comment

noroadsleft left a comment

Choose a reason for hiding this comment

noroadsleft commented May 4, 2020

noroadsleft commented May 4, 2020

yyc commented Jul 5, 2019 •

edited by noroadsleft

Loading

ghost commented Jul 6, 2019 •

edited by ghost

Loading

yyc May 3, 2020 •

edited

Loading